Conversation
|
add reviewers pls |
densmirn
left a comment
densmirn
left a comment
There was a problem hiding this comment.
I would be good to keep code in a single style. I mean using either single or double quotes for strings everywhere.
| -------- | ||
| .. literalinclude:: ../../../examples/dataframe/dataframe_iloc.py | ||
| :language: python | ||
| :lines: 27- |
There was a problem hiding this comment.
| :lines: 27- | |
| :lines: 36- |
Please check the documentation visually.
| results.append(result_c) | ||
| index.append(c) | ||
| data = ', '.join(col for col in results) | ||
| func_lines += [f" return pandas.Series(data=[{data}], index={index}, name=str({name}))", |
There was a problem hiding this comment.
Shouldn't you join index as well as you joined results? Why not insert name directly without type casting?
| """ | ||
| func_lines = ['def _df_getitem_slice_iloc_impl(self, idx):'] | ||
| results = [] | ||
| index = 'self._dataframe.index[idx]' |
There was a problem hiding this comment.
index is used in a single place, maybe not define the variable and use its value directly?
| def df_name_codegen_iloc(self): | ||
| if isinstance(self.index, types.NoneType): | ||
| return 'idx' | ||
|
|
||
| return 'self._dataframe._index[idx]' |
There was a problem hiding this comment.
Looks like the function should be used only ones. So maybe to not define this one and use the code directly?
There was a problem hiding this comment.
The function is used 4 times in all implementations.
| if ( | ||
| isinstance(idx, (types.List, types.Array)) and | ||
| isinstance(idx.dtype, (types.Boolean, bool)) | ||
| ): |
There was a problem hiding this comment.
| if ( | |
| isinstance(idx, (types.List, types.Array)) and | |
| isinstance(idx.dtype, (types.Boolean, bool)) | |
| ): | |
| if (isinstance(idx, (types.List, types.Array)) and | |
| isinstance(idx.dtype, (types.Boolean, bool))): |
| _func_name = 'Attribute iloc().' | ||
|
|
||
| if not isinstance(self, DataFrameType): | ||
| raise TypingError('{} The object must be a pandas.dataframe. Given: {}'.format(_func_name, self)) |
sdc/tests/test_dataframe.py
Outdated
| for n in cases_n: | ||
| for k in cases_n[::-1]: |
There was a problem hiding this comment.
You could combine the loops via itertools.product.
sdc/tests/test_dataframe.py
Outdated
| with self.assertRaises(IndexingError) as raises: | ||
| sdc_func(df) |
There was a problem hiding this comment.
I propose to combine all the tests with such error to a single one because of significant common part. I mean just join the code.
5 participants
No description provided.